Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BlockSchema HTTP client and struct model #180

Merged
merged 8 commits into from
May 31, 2024
Merged

Conversation

mitchnielsen
Copy link
Contributor

Closes #172

@mitchnielsen
Copy link
Contributor Author

Was getting an error from pre-commit:

$ g cm
Terraform fmt........................................(no files to check)Skipped
go fmt...................................................................Passed
go vet...................................................................Passed
go imports...............................................................Passed
validate toml........................................(no files to check)Skipped
golangci-lint............................................................Failed
- hook id: golangci-lint
- exit code: 1

internal/client/block_documents.go:178:46: the given struct should be annotated with the `json` tag (musttag)
        if err := json.NewDecoder(resp.Body).Decode(&blockDocumentAccess); err != nil {
                                                    ^

I suspect this came from #178, but surprised pre-commit / CI didn't complain about it.

Do you get this error locally, too @parkedwards? I did kind of wonder how the unmarshaling would work since we don't have struct tags on those types 🤔

@mitchnielsen mitchnielsen self-assigned this May 31, 2024
@mitchnielsen mitchnielsen added the enhancement New feature or request label May 31, 2024
@parkedwards
Copy link
Contributor

@mitchnielsen what the heck - so when I was dev-ing on #178 , I did run into this linting error

the issue is actually with this struct, but it's (1) nested in the BlockDocumentAccess, (2) I added a nolint comment, and (3) the BlockDocumentAccess struct does have struct tags

i think i upgraded my golangci-lint binary to get around this - what version are you running?

➜ golangci-lint --version
golangci-lint has version 1.59.0 built with go1.22.3 from 2059b18 on 2024-05-25T11:38:08Z

@mitchnielsen
Copy link
Contributor Author

i think i upgraded my golangci-lint binary to get around this - what version are you running?

Ah interesting, I have this version from Homebrew (which it says is the latest available to it):

golangci-lint has version 1.58.1 built with go1.22.3 from dc28153 on 2024-05-07T23:17:27Z

I've been thinking we might benefit from using asdf or mise so we can track the tool versions in a file that we commit to the repo so we're all on the same page.

@mitchnielsen mitchnielsen marked this pull request as ready for review May 31, 2024 16:18
@mitchnielsen mitchnielsen requested review from gabcoyne and a team as code owners May 31, 2024 16:18
@mitchnielsen
Copy link
Contributor Author

https://golangci-lint.run/product/changelog/#1582:

musttag: from 0.12.1 to 0.12.2

https://github.com/go-simpler/musttag/releases/tag/v0.12.2:

go-simpler/musttag@5b5f6e3 fix: check whether a pointer-type implements a Marshaler interface (go-simpler/musttag#94)

That seems to be what fixed it, since updating to golangci-lint 1.59.0 fixed that musttag check 👍🏼

@parkedwards
Copy link
Contributor

ok phew, my sanity is restored 💆

@mitchnielsen mitchnielsen temporarily deployed to Acceptance Tests May 31, 2024 17:16 — with GitHub Actions Inactive
@github-actions github-actions bot added the docs label May 31, 2024
Renames the argument to match the type.

Co-authored-by: Edward Park <[email protected]>
@mitchnielsen mitchnielsen temporarily deployed to Acceptance Tests May 31, 2024 17:21 — with GitHub Actions Inactive
Rather than inheriting the fields from BlockType into BlockSchema,
we want to actually embed a BlockType into BlockSchema. This fixes that
by typing that field as BlockType and specifying the struct tag for it.
@mitchnielsen mitchnielsen temporarily deployed to Acceptance Tests May 31, 2024 20:18 — with GitHub Actions Inactive
@mitchnielsen mitchnielsen merged commit c935e7a into main May 31, 2024
7 checks passed
@mitchnielsen mitchnielsen deleted the 172-add-blockschema branch May 31, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blocks Support: Add BlockSchema HTTP client + struct model
2 participants